Skip to content

Conversation

@iisyos
Copy link
Contributor

@iisyos iisyos commented Oct 29, 2025

Summary

This PR adds type declarations to the scalar type class stub generated by the make:graphql:scalar command.

The ScalarType class uses the NamedTypeImplementation trait, which declares the following typed properties:

public string $name;
public ?string $description;

https://github.com/webonyx/graphql-php/blob/master/src/Type/Definition/NamedTypeImplementation.php#L10-L12

However, the current scalar.stub generates classes without type declarations, causing the following Fatal errors:

Fatal error: Type of SomeScalarType::$name must be string 
Fatal error: Type of SomeScalarType::$description must be ?string 

Added type declarations to the stub file src/Console/stubs/scalar.stub so that generated scalar type classes satisfy the parent class's type requirements:

+ public string $name = 'DummyClass';
- public $name = 'DummyClass';

+ public ?string $description = '';
- public $description = '';

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Misc. change (internal, infrastructure, maintenance, etc.)

Checklist:

  • Existing tests have been adapted and/or new tests have been added
  • Add a CHANGELOG.md entry
  • Update the README.md
  • Code style has been fixed via composer fix-style

Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Is it possible to cover this with a test (generating an actual stub and somehow invoke/lint it?)

@iisyos
Copy link
Contributor Author

iisyos commented Nov 5, 2025

@mfn
Thank you for reviewing!
I covered this change by updating the unit test. (26393b9)
Could you please review again?

Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mfn mfn merged commit 8be5502 into rebing:master Nov 5, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants